Skip to content

feat: add 'app unlink' command #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mwbrooks merged 15 commits into from
Nov 27, 2025
Merged

feat: add 'app unlink' command #266

mwbrooks merged 15 commits into from
Nov 27, 2025

Conversation

@srtaalej
Copy link
Contributor

@srtaalej srtaalej commented Nov 13, 2025 •
edited by mwbrooks
Loading

Summary

This PR adds the app unlink command to the CLI.

Example

$ slack app link
  ✔ Selected team: My Workspace (T0123456)
  ✔ Enter app ID: A999PROD123  # oops, linked wronged app
  ✓ App linked successfully

$ slack app unlink
  ✔ Select app: A999PROD123 (My Workspace - Deployed)
  ⚠️  App (A999PROD123) will be removed from this project
      The app will not be deleted from Slack
      You can re-link it later with 'slack app link'
  ✔ Are you sure? Yes

  ✓ App unlinked from project

Preview

2025-11-27-app-unlink.mov

Requirements

@srtaalej srtaalej self-assigned this Nov 13, 2025
@srtaalej srtaalej requested a review from a team as a code owner November 13, 2025 23:05
@srtaalej srtaalej added enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment labels Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 93.50649% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.73%. Comparing base (8973fda) to head (cac1223).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
cmd/app/unlink.go 93.33% 3 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #266      +/-   ##
==========================================
+ Coverage   64.58%   64.73%   +0.14%     
==========================================
  Files         212      213       +1     
  Lines       17505    17582      +77     
==========================================
+ Hits        11306    11381      +75     
- Misses       5131     5133       +2     
  Partials     1068     1068

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zimeg
zimeg previously requested changes Nov 19, 2025
View reviewed changes
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srtaalej Super nice balance to the changes of the link command! This is working well for me but I'm requesting a few changes below:

  • Inlining the "RunE" logic: As much as possible, I think we'd like to perform most logic within the UnlinkCommandRunE function for quick parsing.
  • Style and outputs: Handfuls of quibble on consistent practices we're working toward but haven't solidified in documentation... I hope these comments are found well and with meaning!
  • Cancelling requests: I noticed unexpected output that's perhaps related to the above points, but we'll want to make sure this case returns as expected too!

Hoping to continue testing this soon! Let me know if I can review more whenever!

cmd/app/unlink.go
Comment on lines +88 to +94
if !proceed {
clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{
Emoji: "thumbs_up",
Text: "Your app will not be unlinked",
}))
return types.App{}, nil
}
Copy link
Member

@zimeg zimeg Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚧 issue: The following outputs appear after selecting "no" for me:

? Are you sure you want to unlink this app? No

👍 Your app will not be unlinked


🔓 App Unlinked
   Removed app  from project
   Team:

👁️‍🗨️ suggestion: Let's inline as much as possible to be within UnlinkCommandRunE to avoid jumping between functions!

Copy link
Member

@mwbrooks mwbrooks Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ This is now resolved:

Image

srtaalej and others added 11 commits November 20, 2025 11:27
@srtaalej @zimeg
Update cmd/app/unlink.go
e0bfd0d 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
7580cca 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
59319b0 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
9716ec1 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
fba3160 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
e57a228 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
200e270 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
7dfc567 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej @zimeg
Update cmd/app/unlink.go
0444adf 
Co-authored-by: Eden Zimbelman <[email protected]>
@srtaalej
fix: bug in unlink logic
10eb0b9
@mwbrooks
Merge branch 'main' into ale-feat-unlink-cmd
3806607
@mwbrooks mwbrooks changed the title feat: app unlink command feat: add 'app unlink' command Nov 26, 2025
@mwbrooks mwbrooks added this to the Next Release milestone Nov 26, 2025
mwbrooks
mwbrooks approved these changes Nov 26, 2025
View reviewed changes
Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fantastic work on adding your first complete command @srtaalej 👏🏻

🧪 Solid test coverage on this PR and manually testing works well, including edge-cases.

✏️ I left a few minor suggestions. After we bring them in, let's record a small video showing this feature in action.

cmd/app/unlink.go
Comment on lines +88 to +94
if !proceed {
clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{
Emoji: "thumbs_up",
Text: "Your app will not be unlinked",
}))
return types.App{}, nil
}
Copy link
Member

@mwbrooks mwbrooks Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ This is now resolved:

Image

@mwbrooks mwbrooks requested a review from zimeg November 27, 2025 19:50
@mwbrooks mwbrooks dismissed zimeg’s stale review November 27, 2025 19:51

The changes have been addressed and the reviewer is on vacation - I've confirmed that the changes look good.

@mwbrooks mwbrooks merged commit f768cc2 into main Nov 27, 2025
8 checks passed
@mwbrooks mwbrooks deleted the ale-feat-unlink-cmd branch November 27, 2025 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mwbrooks mwbrooks mwbrooks approved these changes

@zimeg zimeg Awaiting requested review from zimeg

Assignees

@srtaalej srtaalej

Labels

enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment

Projects

None yet

Milestone

v3.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@srtaalej @mwbrooks @zimeg